NOISSUE - Fix: Clients, Channels roles initialization and Channels connection Authz #2663
Conversation
arvindh123
force-pushed
the
remove_domain_user
branch
2 times, most recently
from
85b38df to
f4c1d1b
Compare
| @@ -40,8 +40,8 @@ type service struct { | |||
|
|
|||
| var _ Service = (*service)(nil) | |||
|
|
|||
| func New(repo Repository, policy policies.Service, idProvider supermq.IDProvider, clients grpcClientsV1.ClientsServiceClient, groups grpcGroupsV1.GroupsServiceClient, sidProvider supermq.IDProvider) (Service, error) { | |||
| rpms, err := roles.NewProvisionManageService(policies.ChannelType, repo, policy, sidProvider, AvailableActions(), BuiltInRoles()) | |||
| func New(repo Repository, policy policies.Service, idProvider supermq.IDProvider, clients grpcClientsV1.ClientsServiceClient, groups grpcGroupsV1.GroupsServiceClient, sidProvider supermq.IDProvider, availableActions []roles.Action, builtInRoles map[roles.BuiltInRoleName][]roles.Action) (Service, error) { | |||
There was a problem hiding this comment.
This signature looks messy. We have id provider twice, and we have way too many params. Can we group roles and actions (maybe something else as well) to a new config struct?
There was a problem hiding this comment.
This is still not addressed.
There was a problem hiding this comment.
Roles need short ID provider, but now we are using uuid for role , it is making the url path longer.
Already we are domain id and entity id in url.
Example requests:
For instance if Client id is 87714e08-3087-476e-8a12-c7bee595b31d client role id is client_4tuyg19Dwd6AzwmnbWr3v6Wm and domain id is 960c325a-c973-45f8-9c0b-f9f686941c7b
-
Get All actions of client
Method: GET URL:http://localhost/960c325a-c973-45f8-9c0b-f9f686941c7b/clients/87714e08-3087-476e-8a12-c7bee595b31d/roles/client_4tuyg19Dwd6AzwmnbWr3v6Wm/actions -
Add All actions
Method: POST URL:http://localhost/960c325a-c973-45f8-9c0b-f9f686941c7b/clients/87714e08-3087-476e-8a12-c7bee595b31d/roles/client_4tuyg19Dwd6AzwmnbWr3v6Wm/actions -
Delete actions
Method: POST URL:http://localhost/960c325a-c973-45f8-9c0b-f9f686941c7b/clients/87714e08-3087-476e-8a12-c7bee595b31d/roles/client_4tuyg19Dwd6AzwmnbWr3v6Wm/actions/delete -
Delete all actions
Method: POST URL:http://localhost/960c325a-c973-45f8-9c0b-f9f686941c7b/clients/87714e08-3087-476e-8a12-c7bee595b31d/roles/client_4tuyg19Dwd6AzwmnbWr3v6Wm/actions/delete-all -
Get All members
Method: GET URL:http://localhost/960c325a-c973-45f8-9c0b-f9f686941c7b/clients/87714e08-3087-476e-8a12-c7bee595b31d/roles/client_4tuyg19Dwd6AzwmnbWr3v6Wm/members -
Add All members
Method: POST URL:http://localhost/960c325a-c973-45f8-9c0b-f9f686941c7b/clients/87714e08-3087-476e-8a12-c7bee595b31d/roles/client_4tuyg19Dwd6AzwmnbWr3v6Wm/members -
Delete members
Method: POST URL:http://localhost/960c325a-c973-45f8-9c0b-f9f686941c7b/clients/87714e08-3087-476e-8a12-c7bee595b31d/roles/client_4tuyg19Dwd6AzwmnbWr3v6Wm/members -
Delete all members
Method: POST URL:http://localhost/960c325a-c973-45f8-9c0b-f9f686941c7b/clients/87714e08-3087-476e-8a12-c7bee595b31d/roles/client_4tuyg19Dwd6AzwmnbWr3v6Wm/members/delete-all
Here Role ID : client_4tuyg19Dwd6AzwmnbWr3v6Wm is encoded using https://github.com/sqids/sqids-go
The code is here https://github.com/absmach/supermq/blob/main/pkg/sid/sid.go
So something like this https://github.com/teris-io/shortid we need
Because of licenses i could decide which one to use
Related to AvailableActions and BuiltInRoles could not be combined because they are using for different purpose.
AvailableActions is list of string which helps user to understand which actions can be added to role.
BuiltInRole contains map of built in role with respective actions , which will automatically added to entity during it creation process
arvindh123
force-pushed
the
remove_domain_user
branch
from
f4c1d1b to
936475e
Compare
channels/api/grpc/request.go
Outdated
| "github.com/absmach/supermq/pkg/policies" | ||
| ) | ||
|
|
||
| var errDomainID = errors.New("domain id required for user type") |
There was a problem hiding this comment.
Let's make messages less cryptic: domain ID required for users.
| @@ -40,8 +40,8 @@ type service struct { | |||
|
|
|||
| var _ Service = (*service)(nil) | |||
|
|
|||
| func New(repo Repository, policy policies.Service, idProvider supermq.IDProvider, clients grpcClientsV1.ClientsServiceClient, groups grpcGroupsV1.GroupsServiceClient, sidProvider supermq.IDProvider) (Service, error) { | |||
| rpms, err := roles.NewProvisionManageService(policies.ChannelType, repo, policy, sidProvider, AvailableActions(), BuiltInRoles()) | |||
| func New(repo Repository, policy policies.Service, idProvider supermq.IDProvider, clients grpcClientsV1.ClientsServiceClient, groups grpcGroupsV1.GroupsServiceClient, sidProvider supermq.IDProvider, availableActions []roles.Action, builtInRoles map[roles.BuiltInRoleName][]roles.Action) (Service, error) { | |||
There was a problem hiding this comment.
This is still not addressed.
dborovcanin
force-pushed
the
remove_domain_user
branch
from
936475e to
e1a0795
Compare
arvindh123
force-pushed
the
remove_domain_user
branch
from
e1a0795 to
215c525
Compare
Signed-off-by: Arvindh <[email protected]>
arvindh123
force-pushed
the
remove_domain_user
branch
from
215c525 to
170da0a
Compare
What type of PR is this?
What does this do?
Which issue(s) does this PR fix/relate to?
Have you included tests for your changes?
Did you document any new/modified feature?
Notes